-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixing the occasional errors in tests #63
Fixing the occasional errors in tests #63
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #63 +/- ##
=======================================
Coverage 86.29% 86.29%
=======================================
Files 7 7
Lines 1138 1138
Branches 187 187
=======================================
Hits 982 982
Misses 121 121
Partials 35 35 ☔ View full report in Codecov by Sentry. |
4c9c309
to
f87c2ff
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there's better ways to do these fixes
Strange, somehow the table packing and unpacking have found their way back into this repo... I will look into this. |
For some reason |
As best as I can work out, We stopped using it in #15 , which was merged. However, there also exists both #14 and #17 , both of which did delete the code but neither of them were merged. There's no notes to say why that may be. It would be clearer if we don't delete that code in this PR - it's an entirely unrelated change to fixing the tests. |
dcb689a
to
a9736d4
Compare
I've cherry-picked the Docs is failing due to the warning
this is occurring on main too. We should fix this on another branch (pr here). |
Increased the amount of time in before SEQ.TABLE is updated panda-side in a test, changed test_create_softioc_system so that it checks the SEQ.TABLE fields first.
Changed tests to use mocked pandas which don't internally change table values throughout the tests.
…side test_non_defined_seq_table_can_be_added_to_panda_side will rarely update the panda in the subprocess before the test process has started. To fix this the panda will wait 50 updates before it changes SEQ3 rather than 10.
02cb078
to
532d964
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
A couple of tests fail occasionally:
test_create_softioc_system
sometimes cagetsSEQ.TABLE
fields part way through the panda-side update ofSEQ.TABLE
. To fix this, the caget has been moved to the start of the test and more time has been added beforeSEQ.TABLE
is updated in the mocked panda.test_table_column_info
would fail becauseTIMEOUT
, to fix the latter we used a different mocked panda which doesn't update its tables. We've also changed to the asyncio p4p client.test_non_defined_seq_table_can_be_added_to_panda_side
this was because very rarely the panda subprocess was too fast and had already changedSEQ3
to have values.SEQ3
rather than 10.Tested locally with
to ensure no failures,
tested on the CI 10 times.